Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

• [ENH]: soft delete databases, add FinishDatabaseDeletion gRPC method to hard delete databases #4627
• [ENH]: perform collection hard deletes from garbage collector #4605
• [ENH]: sysdb changes to support moving collection hard deletes to garbage collector #4607
• [ENH]: add validation during garbage collection for empty file paths #4586
• [ENH]: garbage collector v2 orchestrator (supports forking) #4468 👈 (View in Graphite)
• [ENH]: allow deleting v0 from version file & correctly propagate errors on DeleteCollectionVersion #4579
• [ENH]: add operator for garbage collection to list all files used by collection version #4466
• [ENH]: add operator to compute versions to garbage collect from version graph #4464
• [ENH]: add orchestrator to construct version graph for garbage collection #4463
• [ENH]: add batch get version file paths method to Sysdb #4432 : 1 other dependent PR (#4447 )
• [ENH]: SysDb should return lineage, version file paths and root collection ID on collections #4557
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Garbage Collector V2 Orchestrator with Fork Tree Support and Proptest-Based Validation

This PR introduces a new, extensive, and modularized garbage collector orchestrator (garbage_collector_orchestrator_v2) in Rust that can handle collection version fork trees. Accompanied by a comprehensive property-based testing framework (proptest), the design validates correctness under a wide variety of complex version/fork scenarios. The orchestrator collaborates with modular operator components, and is currently only utilized by the test suite (not active production code), enabling rigorous validation before production rollout.

Key Changes:
• Addition of garbage_collector_orchestrator_v2.rs, a stand-alone GC orchestrator that supports version fork trees across collections.
• Complete module for property-based testing: reference model, arbitrary transition/graph generators, and a state machine test harness.
• Significant supporting test infrastructure: segment file strategies, proptest helpers, and detailed invariants/stats tracking.
• Refinements in library/operator interfaces (notably for delete_unused_files, list_files_at_version) to accommodate new GC orchestrator needs (parameter corrections, clear semantics, vector types, tenant-identification).
• Build and test updates (Cargo.toml, Cargo.lock) to support new dependencies like tracing-subscriber.
• Some refactorings and cleanup in existing operator components for compatibility and testability.

Affected Areas:
• rust/garbage_collector/src/garbage_collector_orchestrator_v2.rs (new orchestrator core)
• rust/garbage_collector/tests/proptest_helpers/* (test infrastructure)
• rust/garbage_collector/src/operators/* (GC-related operators refined)
• rust/garbage_collector/tests/prop_test_garbage_collection.rs (property-based top-level test)
• Cargo.toml/Cargo.lock
• system orchestrator trait defaulting (on_start/initial_tasks signature changes)
• blockstore sparse_index_test_utils

Potential Impact:

Functionality: No production functionality is changed, as GC v2 is not yet wired in; introduces rigorous GC/retention logic for potential future use.

Performance: No impact on runtime; test code is more thorough and possibly slower, but relegated to CI/testing.

Security: No changes.

Scalability: New test harness can validate GC correctness over arbitrarily large and complex version trees, improving confidence in future scalability.

Review Focus:
• Correctness of the garbage collector orchestrator logic for forked version trees.
• Soundness and thoroughness of the proptest state machine and invariants.
• Operator interface changes (delete_unused_files, list_files_at_version): review for breaking changes or backward compatibility.
• Test isolation: no accidental activation of new orchestrator in non-test code.
• Clarity/documentation within orchestrator and test-helper modules.

This summary was automatically generated by @propel-code-bot

Merge activity

• May 23, 10:54 PM UTC: @codetheweb merged this pull request with Graphite.